Skip to content

fix: add timeout to streaming subprocess.run() in base.py - #3967

Open
Quratulain-bilal wants to merge 1 commit into
github:mainfrom
Quratulain-bilal:fix/streaming-subprocess-timeout
Open

fix: add timeout to streaming subprocess.run() in base.py#3967
Quratulain-bilal wants to merge 1 commit into
github:mainfrom
Quratulain-bilal:fix/streaming-subprocess-timeout

Conversation

@Quratulain-bilal

Copy link
Copy Markdown
Contributor

Problem

The streaming branch of dispatch_command() called subprocess.run() without a timeout, which could hang indefinitely if the child process stalls.

Fix

Apply the same timeout parameter (default 600s) as the non-streaming branch, with proper TimeoutExpired handling.

…mmand

The streaming branch of dispatch_command() called subprocess.run()
without a timeout, which could hang indefinitely if the child process
stalls. Now uses the same timeout parameter (default 600s) as the
non-streaming branch, with proper TimeoutExpired handling.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds timeout protection to streaming integration commands, preventing stalled subprocesses from hanging indefinitely.

Changes:

  • Passes the configured timeout to streaming subprocesses.
  • Converts timeout failures into exit code 124 with a clear error.
Show a summary per file
File Description
src/specify_cli/integrations/base.py Adds streaming timeout enforcement and handling.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

  • Files reviewed: 1/1 changed files
  • Comments generated: 1
  • Review effort level: Balanced

exec_args,
text=True,
cwd=cwd,
timeout=timeout,
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants